-
-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhance Coffee Cup Image with Parallax Effect #54
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes in this pull request enhance the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Thank you for submitting your pull request! 🙌 We'll review it as soon as possible. In the meantime, please ensure that your changes align with our CONTRIBUTING.md. If there are any specific instructions or feedback regarding your PR, we'll provide them here. Thanks again for your contribution! 😊 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
frontend/src/components/ui/Landing.jsx (1)
Line range hint
56-78
: Remove Unused Commented-Out CodeThe block of commented-out code is no longer needed and can be removed to keep the codebase clean and maintainable.
Apply this diff to remove the unused code:
<section className="flex flex-row justify-center items-center p-32" > - {/* <div className="w-3/5 p-28 mt-20"> - <h1 className="text-8xl font-bold text-black "> - Our name says it all! - </h1> - <p className="text-black text-2xl"> - Founder, Jonathan Li, shares a passion for board games, boba, and - delicious food, so he combined them all to become Sip & Play, Park - Slope’s first board game cafe. It is a straightforward concept: come - in with your friends and family to play any board game from our - library of 300+ games! We hope when you visit, you also enjoy our - coffee, espresso, boba, sandwiches, and snacks! - </p> - <ul className="text-xl mt-4"> - <li>New opening hours:</li> - <li>Sunday: 10am-11pm</li> - <li>Mon-Thurs: 11am-11pm</li> - <li>Fri: 11am-midnight</li> - <li>Sat: 10am-midnight</li> - </ul> - <button className="p-2 border-2 border-slate-500">Learn more↗️</button> - </div> */}Removing commented-out code improves readability and makes maintenance easier.
const parallaxEffect = { | ||
x: mousePosition.x / 100, | ||
y: mousePosition.y / 100, | ||
|
||
|
||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adjust parallaxEffect
Implementation for Correct Motion
The parallaxEffect
object currently sets x
and y
properties based on the mouse position and applies them directly to the style
prop of the motion.img
. However, in Framer Motion, to animate positional properties like x
and y
, these should be motion values or controlled via the animate
prop. Using plain numbers in the style
prop won't produce the intended animation effect.
Consider using motion values or updating the animate
prop to achieve the desired parallax effect. Here's how you can modify the code:
+import { useMotionValue, useTransform } from 'framer-motion';
...
+const x = useMotionValue(0);
+const y = useMotionValue(0);
...
const handleMouseMove = (e) => {
- const { clientX, clientY } = e;
- setMousePosition({ x: clientX, y: clientY });
+ const { clientX, clientY } = e;
+ // Calculate the offset from the center of the screen
+ const offsetX = (clientX - window.innerWidth / 2) / 20;
+ const offsetY = (clientY - window.innerHeight / 2) / 20;
+ x.set(offsetX);
+ y.set(offsetY);
};
-const parallaxEffect = {
- x: mousePosition.x / 100,
- y: mousePosition.y / 100,
-};
...
<motion.img
src={coffecup}
alt="Coffee Cup"
className="cursor-pointer"
variants={parallaxVariants}
initial="initial"
animate="animate"
whileHover={{ scale: 1.1 }}
transition={transition}
- style={parallaxEffect }
+ style={{ x, y }}
/>
This approach uses useMotionValue
to create reactive x
and y
values that Framer Motion can animate efficiently.
Also applies to: 89-91
const handleMouseMove = (e) => { | ||
const { clientX, clientY } = e; | ||
setMousePosition({ x: clientX, y: clientY }); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Throttle handleMouseMove
for Improved Performance
Updating state on every mouse move event can lead to performance issues due to excessive re-renders. To optimize performance, consider throttling the handleMouseMove
function.
You can use requestAnimationFrame
to limit updates to once per animation frame:
+import { useRef } from 'react';
export default function Landing() {
const [mousePosition, setMousePosition] = useState({ x: 0, y: 0 });
+ const animationFrame = useRef(null);
const handleMouseMove = (e) => {
+ if (animationFrame.current === null) {
+ animationFrame.current = requestAnimationFrame(() => {
const { clientX, clientY } = e;
setMousePosition({ x: clientX, y: clientY });
+ animationFrame.current = null;
+ });
+ }
};
This ensures that state updates occur at most once per frame, reducing the render load.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const handleMouseMove = (e) => { | |
const { clientX, clientY } = e; | |
setMousePosition({ x: clientX, y: clientY }); | |
}; | |
import { useRef } from 'react'; | |
export default function Landing() { | |
const [mousePosition, setMousePosition] = useState({ x: 0, y: 0 }); | |
const animationFrame = useRef(null); | |
const handleMouseMove = (e) => { | |
if (animationFrame.current === null) { | |
animationFrame.current = requestAnimationFrame(() => { | |
const { clientX, clientY } = e; | |
setMousePosition({ x: clientX, y: clientY }); | |
animationFrame.current = null; | |
}); | |
} | |
}; |
@anshi05 can you some 3d type effect on it you can use aos.js |
Ok let me try if I can do that . Though I've never done that before. |
@anshi05 sure |
@anshi05 is attempting to deploy a commit to the bunty's projects Team on Vercel. A member of the Team first needs to authorize it. |
I have added the fading animation to the text "PLAYCAFE". So now the coffee mug is visible and then the text comes on top of it after few seconds enhancing the overall user experience. BoardGame.Cafe.-.Opera.2024-10-07.01-18-54.mp4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
frontend/package.json (1)
15-15
: LGTM! Consider adding AOS types for better development experience.The addition of the AOS library (^2.3.4) aligns well with the PR objectives to implement scroll animations. This change is appropriately placed and maintains the alphabetical order of dependencies.
For improved TypeScript support and better IDE intellisense, consider adding the
@types/aos
package to yourdevDependencies
. You can do this by running:npm install --save-dev @types/aos
This will enhance the development experience when working with AOS in your TypeScript/JavaScript files.
frontend/src/components/ui/Landing.jsx (3)
87-99
: Good progress on parallax effect, consider further optimizationThe implementation of the coffee cup image with Framer Motion is a good step towards achieving the desired parallax effect. The use of animation variants, transitions, and hover effects enhances the user experience.
However, as mentioned in previous comments, consider optimizing the parallax effect by using motion values instead of directly applying the
parallaxEffect
to the style prop. This would result in smoother animations and better performance.Refer to the earlier comments for detailed suggestions on implementing motion values for the parallax effect.
104-117
: LGTM: AOS attributes for text animationThe addition of AOS attributes to the "PLAY" and "CAFE" text spans is a great way to enhance the visual appeal of the landing page. The use of different delays for each span creates a nice staggered animation effect.
Consider extracting the AOS-related className and data attributes into separate constants or a utility function to improve code readability and maintainability, especially if you plan to use similar animations elsewhere in the component.
Line range hint
1-122
: Overall feedback: Good progress, with room for optimizationThe changes in this PR significantly enhance the visual appeal and interactivity of the landing page, aligning well with the stated objectives. The implementation of the parallax effect, AOS animations, and Framer Motion contribute to a more engaging user experience.
However, there are opportunities for further optimization:
- Implement throttling for the mouse move handler to improve performance.
- Use Framer Motion's motion values for a smoother parallax effect.
- Consider extracting repeated animation attributes into constants or utility functions for better maintainability.
These optimizations will further improve the code quality and performance of the component. Great work on enhancing the landing page!
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- frontend/package.json (1 hunks)
- frontend/src/components/ui/Landing.jsx (3 hunks)
🧰 Additional context used
🔇 Additional comments (5)
frontend/src/components/ui/Landing.jsx (5)
1-17
: LGTM: New imports and animation constantsThe new imports and constants are well-structured and necessary for the implemented features. The use of framer-motion and AOS libraries aligns with the PR objectives of enhancing the visual appeal of the landing page.
20-25
: LGTM: AOS initializationThe AOS library is correctly initialized using the useEffect hook. The settings are appropriate for achieving smooth scroll animations, and the hook is set to run only once on component mount, which is the correct approach.
42-42
: LGTM: Improved section stylingThe increased margin-bottom and addition of overflow-hidden improve the layout and prevent potential issues with content overflow. These changes contribute to a cleaner visual presentation of the landing page.
27-32
: 🛠️ Refactor suggestionConsider implementing throttling for mouse move handler
The mouse position tracking is implemented correctly. However, as mentioned in a previous review comment, consider implementing throttling for the
handleMouseMove
function to optimize performance. This will reduce the frequency of state updates and re-renders, especially during rapid mouse movements.
34-39
: 🛠️ Refactor suggestionImplement motion values for smooth parallax effect
As pointed out in a previous review comment, the current implementation of
parallaxEffect
may not produce the intended animation. Consider implementing motion values using Framer Motion'suseMotionValue
anduseTransform
hooks for a smoother parallax effect.Refer to the previous comment for a detailed implementation suggestion using motion values.
This PR has been automatically closed due to inactivity from the owner for 3 days. |
Fixes #52
It introduces enhancements to the Landing component by applying a parallax effect to the coffee cup image. The key changes include:
Changes Made:
Updated the parallaxEffect to combine both translation and rotation in the transform property of the motion.img element.
Ensured that the rotation does not interfere with the parallax effect, maintaining the visual consistency of the design.
Testing:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes